fix: parse IP address without the port number#3865
Conversation
IP address type doesn't accept port numbers in elastic. Refs: HL-1614
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
| client_ip = client_ip.lstrip("[").split("]")[0] | ||
| elif "." in client_ip and client_ip.count(":") == 1: | ||
| # IPv4 with port | ||
| client_ip = client_ip.split(":")[0] |
There was a problem hiding this comment.
Bug: The port-stripping logic fails for unbracketed IPv4-mapped IPv6 addresses (e.g., ::ffff:192.0.2.1:8000) because it incorrectly checks for a single colon, leaving the port attached.
Severity: MEDIUM
Suggested Fix
Update the logic to correctly identify and strip ports from unbracketed IPv6 addresses. This could involve checking for a dot and multiple colons, then finding the last colon to reliably split the host from the port, regardless of the IP address format.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: backend/shared/shared/audit_log/utils.py#L12-L15
Potential issue: The logic to strip ports from IP addresses does not correctly handle
unbracketed IPv4-mapped IPv6 addresses that contain a port, such as
`::ffff:192.0.2.1:8000`. The condition `elif "." in client_ip and client_ip.count(":")
== 1` fails because such addresses contain multiple colons. As a result, the port is not
stripped. This causes an IP address with a port to be sent to Elasticsearch, which
cannot index it as an `ip` type, leading to the silent failure of audit log persistence
and data loss.
Did we get this right? 👍 / 👎 to inform future reviews.
|
|
YJDH-KESASETELI-API branch is deployed to platta: https://yjdh-kesaseteli-pr3865.api.dev.hel.ninja 🚀🚀🚀 |
|
|
YJDH-HELSINKILISA-API branch is deployed to platta: https://helsinkilisa-pr3865.api.dev.hel.ninja 🚀🚀🚀 |
|
YOUTH branch is deployed to platta: https://nuortenkesaseteli-pr3865.dev.hel.ninja 🚀🚀🚀 |
|
HANDLER branch is deployed to platta: https://kesaseteli-handler-ui-pr3865.dev.hel.ninja 🚀🚀🚀 |
|
EMPLOYER branch is deployed to platta: https://kesaseteli-pr3865.dev.hel.ninja 🚀🚀🚀 |
TestCafe result is success for https://kesaseteli-handler-ui-pr3865.dev.hel.ninja 😆🎉🎉🎉 |
TestCafe result is success for https://nuortenkesaseteli-pr3865.dev.hel.ninja 😆🎉🎉🎉 |
|
APPLICANT is deployed to platta: https://helsinkilisa-ui-pr3865.dev.hel.ninja 🚀🚀🚀 |
TestCafe result is success for https://kesaseteli-pr3865.dev.hel.ninja 😆🎉🎉🎉 |
|
HANDLER branch is deployed to platta: https://helsinkilisa-ui-handler-pr3865.dev.hel.ninja 🚀🚀🚀 |
TestCafe result is success for https://helsinkilisa-ui-pr3865.dev.hel.ninja 😆🎉🎉🎉 |
TestCafe result is success for https://helsinkilisa-ui-handler-pr3865.dev.hel.ninja 😆🎉🎉🎉 |
karisal-anders
left a comment
karisal-anders
left a comment
There was a problem hiding this comment.
I thought that part of get_remote_address function could've been replaced by using ipaddress.ip_address but it doesn't support ports, e.g.:
>>> from ipaddress import ip_address
>>> ip_address("192.168.0.1:1234")
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
File "Python312/Lib/ipaddress.py", line 54, in ip_address
raise ValueError(f'{address!r} does not appear to be an IPv4 or IPv6 address')
ValueError: '192.168.0.1:1234' does not appear to be an IPv4 or IPv6 addressApproval from Kesäseteli side for the shared code part.
2 participants
IP address type doesn't accept port numbers in elastic.
Refs: HL-1614